feat(recording): add automatic cursor overlay - #42
Conversation
|
@shreyaskarnik @jarkkosyrjala please review and approve, waiting for this merge |
|
Thanks a lot for this, @OleksandrKucherenko — really appreciate the contribution! 🙏 This is a clean piece of work: the config → CLI → Merging this now. I did spot a few edge cases around the
Follow-up makes the injection idempotent + body-safe and decouples the paint trigger. Also worth noting for posterity: moving Thanks again! |
|
Follow-up with the fixes is up: #43 — covers all three points above (null |
Follow-up to #42. Three issues in the `framenavigated` reinjection path: 1. `document.body` can be null at navigation commit. Playwright emits `framenavigated` when the nav commits, which can precede the parser producing <body> (a render/parser-blocking resource in <head> widens the window). `cursorHighlight()` ended in `document.body.appendChild`, throwing `TypeError: Cannot read properties of null` — a message that is not in cursor.ts's disposal-error allowlist, so it surfaced as a warning and left the page permanently cursor-less. Injection now defers to DOMContentLoaded when body is absent, with a generation counter so a later call supersedes a queued install. 2. Same-document navigations reinjected needlessly. Playwright dispatches `navigated` from `frameCommittedSameDocumentNavigation` too, so every pushState/hash route change rebuilt the ring: it reset to left:-100px (invisible until the next mouse event) and the replace path removed the element without invoking the `__cleanup` stored on it, leaking a capturing mousemove + click listener per route change (N leaks => N stacked ripples per click). Adds an internal `ensureCursorHighlight()` that no-ops when a ring is present or queued; the replace path now runs the previous cleanup. 3. The paint nudge was chained behind the cursor evaluate. That listener exists to unstick CDP promptly after a navigation, and the evaluate blocks on the new document's execution context. Paint now fires immediately and again once the cursor lands. `ensureCursorHighlight` is deliberately not re-exported from index.ts — public API surface is unchanged. Tests execute the injected browser function against a DOM stub, so the injection logic itself is covered rather than just its arguments. Each new test was verified to fail against the pre-fix implementation.
4 participants
Summary
video.cursorHighlightconfiguration for recording-wide pseudo-cursor feedbackTesting
npx tsc --noEmitnpx vitest run tests/cursor.test.ts tests/config.test.ts tests/record.test.ts tests/narration.test.ts tests/cli.test.tsnpx vitest run tests/pipeline.test.ts -t "forwards automatic cursor highlighting"Closes #37